Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Tweak supported Node.js versions, drop io.js #1579

Merged
merged 1 commit into from
Sep 9, 2015

Conversation

mgol
Copy link
Contributor

@mgol mgol commented Sep 9, 2015

Current setup means karma won't be tested on future updates to the Node v4
line. Those updates are not allowed to introduce breaking changes and only
the newest one is officially supported so that's the one that should be tested.

In the same way, ythe config doesn't contain "0.10.0" or "0.12.0" but
"0.10" & "0.12" so it catches latest updates.

io.js testing has been dropped - io.js has no official support policy as
opposed to Node.js; also, it won't see any major releases.

Refs 6b8d30f

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

I can optionally remove io.js 3 from Travis. It won't be supported for much longer, there's no official support policy for io.js while Node.js 0.10/0.12 will officially be supported for another year and Node 4 for another 2.5 years.

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

Something is broken on master; my changes currently invoke tests on Node 4.0.0 since there is no newer version yet and yet it fails.

@dignifiedquire
Copy link
Member

The integration tests fail on node@4 currently. Need to investigate what's happening there.

Re iojs yes let's drop it, 4.0 has most of it anyway. Could you please also update the doc about what versions we support to mirror these changes please?

@dignifiedquire
Copy link
Member

Damn, looks like because of all the optional dependencies travis is terminating the build, even though it succeeds for me locally.

See https://s3.amazonaws.com/archive.travis-ci.org/jobs/79468450/log.txt

@mgol mgol force-pushed the travis branch 2 times, most recently from d571242 to 93bc91f Compare September 9, 2015 13:49
@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

Re iojs yes let's drop it, 4.0 has most of it anyway. Could you please also update the doc about what versions we support to mirror these changes please?

Done. FAQ & installation docs had conflicting information so I made them consistent. There is duplicated info there, though, maybe on of them could be simplified?

I assumed we'll want to support all LTS releases in "active" support state, I didn't mention the "maintenance" support phase as it receives only security updates so it's mostly for apps already deployed and not modified anymore; new Karma releases shouldn't matter for that (read https://github.com/nodejs/LTS/blob/master/README.md for more info about the plan). Node 0.10 is still popular, though so I made an explicit exception for it. I also assumed we'll want to support the latest stable Node (every other stable release will become an LTS, new stable releases will appear each 6 months, not 6 weeks as it was with io.js).

The expected schedule would look as follows:

  1. Node 5 is released - Karma then supports Node 0.12 (LTS, active phase), 4.x (LTS, active phase) and, as an exception, Node 0.10 (LTS, maintenance phase). It will also support Node 5 (stable).
  2. Node 6 is released - Karma then supports Node 0.12 (LTS, active phase) and 4.x (LTS, active phase). It will also support Node 6 (stable).
  3. Node 7 is released, Node 6 becomes an LTS - Karma then supports Node 4.x (LTS, active phase) and 6.x (LTS, active phase). It will also support Node 7 (stable).
  4. Node 8 is released - Karma then supports Node 4.x (LTS, active phase) and 6.x (LTS, active phase). It will also support Node 8 (stable).

Does that sound OK?

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

I dropped an obsolete Node 0.8 entry from .travis.yml and made npm update only on 0.10 as it has npm@1 which has problems. npm in Node 0.12/4.0 is actively updated so it makes sense to test on the built-in version.

@dignifiedquire
Copy link
Member

Sounds good, but it looks like travis is choking on your change for the TRAVIS_NODE_VERSION check

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

Sounds good, but it looks like travis is choking on your change for the TRAVIS_NODE_VERSION check

Oops. Fixed.

@dignifiedquire dignifiedquire mentioned this pull request Sep 9, 2015
@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

Integration tests pass for me locally on Node 4.0.0 as well (OS X 10.11 public beta 6), although lots of optional deps fail to install in the process.

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

Isn't this the reason for the termination of the build?

The log length has exceeded the limit of 4 Megabytes (this usually means that test suite is raising the same exception over and over).

The build has been terminated.

It seems it's getting killed because it logs too much...

@dignifiedquire
Copy link
Member

It's mostly one, all related to socket.io not upgrading their dependencies, see #1577
The build would pass fine, but npm spits out so much log data about the optional deps failing that travis kills it, so we probably have to live with a failing build on travis for now :(

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

The build would pass fine, but npm spits out so much log data about the optional deps failing that travis kills it, so we probably have to live with a failing build on travis for now :(

We can add v4 to accepted failures temporarily so that build is green. This should be fixed ASAP, though to make sure we don't regress on v4.

Let's hope socketio/socket.io#2228 is resolved soon.

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

Do you want me to add:

matrix:
  allow_failures:
    - node_js: "4"

to .travis.yml for now?

@dignifiedquire
Copy link
Member

Yes let's do that, I'll have to check the travis builds manually to ensure nothing too bad happens

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

Done. I also reduced the duplication in the docs by referring to FAQ from installation notes.

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

https://travis-ci.org/karma-runner/karma/jobs/79486742 failed (Node 0.12), not sure why, I haven't changed anything related. A flake? Could you restart this one job?

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

Never mind, I updated the PR again. README previously referred to http://nodejs.org/download/ but this URL is hardly user-friendly. The main Node.js page is very clear how to download the proper Node for your platform so I linked to that instead.

Note that I changed the recommendation for OS X to use nvm as well. Not sure if that's easier for people than downloading the installer from the official site but the installed version doesn't update and Linux people can download from the site as well so I don't see why we should be inconsistent here - either point to the official site for all people or suggest nvm for both Linux & OS X.

@dignifiedquire
Copy link
Member

Looks good to me, thanks a lot

@dignifiedquire
Copy link
Member

One thing can you remove the (*) from the commit message, I think the parser will choke on that, just chore: Tweak... should be fine

Current setup means karma won't be tested on future updates to the Node v4
line. Those updates are not allowed to introduce breaking changes and only
the newest one is officially supported so that's the one that should be tested.

In the same way, ythe config doesn't contain "0.10.0" or "0.12.0" but
"0.10" & "0.12" so it catches latest updates.

io.js testing has been dropped - io.js has no official support policy as
opposed to Node.js; also, it won't see any major releases.

Refs 6b8d30f
@GitCop
Copy link

GitCop commented Sep 9, 2015

There were the following issues with your Pull Request

  • Commit: ce1d26f
    • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at http://karma-runner.github.io/0.13/dev/git-commit-msg.html


This message was auto-generated by https://gitcop.com

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

One thing can you remove the (*) from the commit message, I think the parser will choke on that, just chore: Tweak... should be fine

Done. I was previously following Angular conventions. :)

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

The previous run was green: https://travis-ci.org/karma-runner/karma/builds/79488599 so hopefully nothing unexpected happens this time. :)

@mgol
Copy link
Contributor Author

mgol commented Sep 9, 2015

It's green. :) Ready to land?

dignifiedquire added a commit that referenced this pull request Sep 9, 2015
Tweak tested/allowed Node.js/io.js versions
@dignifiedquire dignifiedquire merged commit c8b5dc3 into karma-runner:master Sep 9, 2015
@dignifiedquire
Copy link
Member

:octocat: is happy

@mgol mgol deleted the travis branch September 9, 2015 15:49
@mgol mgol changed the title Tweak tested/allowed Node.js/io.js versions chore: Tweak supported Node.js versions, drop io.js Sep 9, 2015
@19h
Copy link

19h commented Sep 10, 2015

Re iojs yes let's drop it, 4.0 has most of it anyway. - nodejs 4 is iojs 4. In fact, node-legacy was moved into a archive repo (https://github.com/nodejs/node-v0.x-archive) and the iojs repo was renamed to node (https://github.com/nodejs/node). That is, I would support iojs 2.5 for a few months (at least we're using it in production until we hit 4.0 with the next major deployment); 3.0 is ABI identical to 4.0 with the difference that v8 4.5 introduced arrow functions.

@mgol
Copy link
Contributor Author

mgol commented Sep 10, 2015

@KenanSulayman io.js 2.x was already not tested so my PR didn't change much in this regard. :) It's also not supported by upstream anymore AFAIK so I'd get off it ASAP.

@mgol
Copy link
Contributor Author

mgol commented Sep 10, 2015

(anyway, as I said, I didn't remove io.js 2 here since it wasn't here in the first place so if you'd like it back, you should open a separate issue)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants